Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Separate config for preview_urls #7227

Merged
merged 3 commits into from
Nov 26, 2024
Merged

Separate config for preview_urls #7227

merged 3 commits into from
Nov 26, 2024

Conversation

taylorlee
Copy link
Contributor

@taylorlee taylorlee commented Nov 11, 2024 •
edited by WalshyDev
Loading

Fixes WC-2914

Add preview_urls toggle to wrangler.toml

The current Preview URLs (beta) feature routes to version preview urls based on the status of the workers_dev config value. Beta users have requested the ability to enable deployment urls and preview urls separately on workers.dev, and the new previews_enabled field of the enable-subdomain API will allow that. This change separates the workers_dev and workers_dev_previews behavior during wrangler triggers deploy and wrangler versions upload. wrangler_dev_previews defaults to true, and does not implicitly depend on routes the way wrangler_dev does.

  • Tests
    • TODO (before merge)
    • Tests included
    • Tests not necessary because:
  • E2E Tests CI Job required? (Use "e2e" label or ask maintainer to run separately)
    • I don't know
    • Required
    • Not required because: workflows does not have them
  • Public documentation

@taylorlee taylorlee requested a review from a team as a code owner November 11, 2024 16:55
@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Nov 11, 2024 •
edited
Loading

🦋 Changeset detected

Latest commit: a0c2336

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
wrangler Minor
@cloudflare/vitest-pool-workers Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

taylorlee
taylorlee commented Nov 11, 2024
View reviewed changes
packages/wrangler/src/triggers/deploy.ts
});
}
if (!deployToWorkersDev && deploymentInSync && routes.length !== 0) {
// TODO is this true? How does last subdomain status affect route confict??
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kept this unchanged (aside from unrolling the conditional branches for easier context readability), but I don't think this branch makes sense anymore.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Nov 11, 2024 •
edited
Loading

A wrangler prerelease is available for testing. You can install this latest build in your project with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12030227543/npm-package-wrangler-7227

You can reference the automatically updated head of this PR with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/7227/npm-package-wrangler-7227

Or you can use npx with this latest build directly:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12030227543/npm-package-wrangler-7227 dev path/to/script.js
Additional artifacts: 
npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12030227543/npm-package-create-cloudflare-7227 --no-auto-update
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12030227543/npm-package-cloudflare-kv-asset-handler-7227
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12030227543/npm-package-miniflare-7227
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12030227543/npm-package-cloudflare-pages-shared-7227
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12030227543/npm-package-cloudflare-vitest-pool-workers-7227
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12030227543/npm-package-cloudflare-workers-editor-shared-7227
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12030227543/npm-package-cloudflare-workers-shared-7227
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12030227543/npm-package-cloudflare-workflows-shared-7227

Note that these links will no longer work once the GitHub Actions artifact expires.

[email protected] includes the following runtime dependencies:

Package Constraint Resolved
miniflare workspace:* 3.20241106.1
workerd 1.20241106.1 1.20241106.1
workerd --version 1.20241106.1 2024-11-06

Please ensure constraints are pinned, and miniflare/workerd minor versions match.

@penalosa
Copy link
Contributor

Sorry to bikeshed on this, but I'm not sure about the name here. I know previews are always on workers.dev for now, but I imagine at some point in the future we might want to support previews on a custom domain/zone the user controls? Is there any reason we can't call this something like worker_previews or preview_urls?

@taylorlee taylorlee force-pushed the tlee/preview-url-separate-subdomain-toggle-2 branch from 6ca4cbf to 40665e2 Compare November 11, 2024 19:53
@taylorlee
Copy link
Contributor Author

Sorry to bikeshed on this, but I'm not sure about the name here. I know previews are always on workers.dev for now, but I imagine at some point in the future we might want to support previews on a custom domain/zone the user controls? Is there any reason we can't call this something like worker_previews or preview_urls?

Definitely open to change! Any of worker_previews, preview_urls, or worker_preview_urls seems reasonable to me. What is your preference?

@korinne
Copy link

korinne commented Nov 12, 2024

After a quick chat with everyone, we decided preview_urls works best. Thanks @penalosa and @taylorlee!

Base automatically changed from tlee/preview-url-separate-subdomain-toggle to main November 13, 2024 16:45
@WalshyDev WalshyDev force-pushed the tlee/preview-url-separate-subdomain-toggle-2 branch from 40665e2 to 01f04ea Compare November 14, 2024 13:42
@WalshyDev WalshyDev added the e2e Run e2e tests on a PR label Nov 14, 2024
@WalshyDev WalshyDev changed the title Separate config for workers_dev_previews Separate config for preview_urls Nov 14, 2024
@WalshyDev WalshyDev force-pushed the tlee/preview-url-separate-subdomain-toggle-2 branch from 01f04ea to 6273a64 Compare November 21, 2024 11:10
@taylorlee @WalshyDev
feat(wrangler): create separate config for workers_dev_previews
ce7540b 
The current Preview URLs (beta) feature routes to version preview urls based on the status of the `workers_dev` config value. Beta users have requested the ability to enable deployment urls and preview urls separately on `workers.dev`, and the new `previews_enabled` field of the enable-subdomain API will allow that. This change separates the `workers_dev` and `workers_dev_previews` behavior during `wrangler triggers deploy` and `wrangler versions upload`. `wrangler_dev_previews` defaults to true, and does not implicitly depend on routes the way `wrangler_dev` does.
@WalshyDev WalshyDev force-pushed the tlee/preview-url-separate-subdomain-toggle-2 branch from 6273a64 to a3ea544 Compare November 25, 2024 14:22
@WalshyDev WalshyDev force-pushed the tlee/preview-url-separate-subdomain-toggle-2 branch from a3ea544 to 6687d0e Compare November 25, 2024 14:37
@WalshyDev WalshyDev merged commit 02a0e1e into main Nov 26, 2024
31 of 32 checks passed
@WalshyDev WalshyDev deleted the tlee/preview-url-separate-subdomain-toggle-2 branch November 26, 2024 12:54
@workers-devprod workers-devprod mentioned this pull request Nov 26, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@WalshyDev WalshyDev WalshyDev approved these changes

@emily-shen emily-shen emily-shen approved these changes

Assignees

@WalshyDev WalshyDev

@penalosa penalosa

Labels
e2e Run e2e tests on a PR
Projects
workers-sdk
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@taylorlee @penalosa @korinne @WalshyDev @emily-shen